Skip to content

HDDS-15205. Cut WritableRPCEngine#10213

Merged
ivandika3 merged 2 commits into
apache:masterfrom
adoroszlai:HDDS-15205
May 9, 2026
Merged

HDDS-15205. Cut WritableRPCEngine#10213
ivandika3 merged 2 commits into
apache:masterfrom
adoroszlai:HDDS-15205

Conversation

@adoroszlai
Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Port from Hadoop: HADOOP-19864. Cut WritableRPCEngine.

Ozone uses ProtobufRpcEngine, never used WritableRPCEngine.

https://issues.apache.org/jira/browse/HDDS-15205

How was this patch tested?

https://github.com/adoroszlai/ozone/actions/runs/25539302248

@adoroszlai adoroszlai self-assigned this May 8, 2026
Copy link
Copy Markdown
Contributor

@ivandika3 ivandika3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM +1. Same as HADOOP-19764 but without additional comments.

@adoroszlai
Copy link
Copy Markdown
Contributor Author

Thanks @ivandika3 for the review. I have noticed a follow-up PR (apache/hadoop#8470), included in another commit here.

@ivandika3
Copy link
Copy Markdown
Contributor

ivandika3 commented May 8, 2026

@adoroszlai The implementation looks good and there is already a +1 so I think it's ok to include in this patch. I'm still +1.

@ivandika3
Copy link
Copy Markdown
Contributor

ivandika3 commented May 8, 2026

@adoroszlai I think we should include the test testUnregisteredRpcKindRejectedWithoutDeserialization.

In the future Ozone might decide to change the Hadoop RPC logic to suite Ozone's need so we need test to catch regressions.

@adoroszlai
Copy link
Copy Markdown
Contributor Author

I think we should include the test testUnregisteredRpcKindRejectedWithoutDeserialization.

Unit tests for Hadoop RPC are not included in Ozone, because RPC was copied from Hadoop 3.2, the last version without ProtobufRpcEngine2, and tests in that version use JUnit4, which we no longer support.

We can attempt to copy and migrate tests in a follow-up.

@ivandika3
Copy link
Copy Markdown
Contributor

@adoroszlai Got it, thanks for the info.

@ivandika3 ivandika3 merged commit 32a88e7 into apache:master May 9, 2026
47 checks passed
@ivandika3
Copy link
Copy Markdown
Contributor

Thanks @adoroszlai for the patch.

@adoroszlai adoroszlai deleted the HDDS-15205 branch May 10, 2026 05:04
@adoroszlai
Copy link
Copy Markdown
Contributor Author

Thanks @ivandika3 for reviewing and merging this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants